New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[New] Add no-arrow-function-lifecycle
rule
#1980
Conversation
@ljharb Thanks for your reviews. I think I have addressed your comments already. Could you check it out again? If you have any concerns, just leave your comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rule seems like it should be autofixable. Can it be?
@ljharb I really keen on your idea but, in my point of view it should be fixed by creator to make sure that they won't repeat it in next time. Beside that, I also figure out some common cases:
To be honest, I didn't find out a good way to replace them with instance method yet. Anyway, I believe I will have a one to help it automatically fixed later after it was merged. |
|
I also thought about it. But, have idea for now. |
The autofix would be so easy if we had AST-driven fixers 🙁 (although I'm guessing there's valid reasons we don't?) |
Honestly, I'm working on this. |
I think |
Any update on this? Thanks |
➕ |
I'm going to add to the motivation for this rule. In my experience the @ngtan // @ljharb This has been idle for a while - but I don't see any outstanding change requests. I'm interested in helping move this forward. Do we need to write a fixer or could that come in a subsequent patch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be just confined to arrow functions; a non-arrow function in a class field should error too.
In other words, let's rename this rule to lifecycle-methods
, and have it enforce that anything with a lifecycle method name is an instance method.
@ngtan any interest in finishing this PR? |
Hi, It has been a long time. I will make time this week to finish the PR. |
Codecov Report
@@ Coverage Diff @@
## master #1980 +/- ##
=======================================
Coverage 97.40% 97.41%
=======================================
Files 111 113 +2
Lines 7444 7463 +19
Branches 2723 2725 +2
=======================================
+ Hits 7251 7270 +19
Misses 193 193
Continue to review full report at Codecov.
|
no-arrow-function-lifecycle
rule
2668b77
to
a9f0b95
Compare
Overview
I would like to add one more rule no-arrow-function-lifecycle which checks that it is not necessary to use arrow function for lifecycle methods and it will affect a bit performance.
Movitation
I did code review some projects and found that our teammates use arrow function for lifecycle methods constantly. Beside that, I also found a ticket related to this issue. See react/issues/10810 for more details.
Consider the below code
Do you need to use arrow function here? I believe that, you will have the same idea that it shouldn't.
I'm eagerly looking forward to seeing your comments.